Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set parameters from file for composable nodes #281

Merged

Conversation

adityapande-1995
Copy link
Contributor

@adityapande-1995 adityapande-1995 commented Oct 26, 2021

This PR aims to extend the SetParametersFromFile launch action to composable nodes as well.

Sample usage :

import launch
from launch_ros.actions import ComposableNodeContainer
from launch_ros.descriptions import ComposableNode
from launch_ros.actions import SetParameter, SetParametersFromFile
from launch.actions import GroupAction

def generate_launch_description():
    container = ComposableNodeContainer(
            name='my_container',
            namespace='',
            package='rclcpp_components',
            executable='component_container',
            composable_node_descriptions=[
                ComposableNode(
                    package='demo_nodes_cpp',
                    plugin='demo_nodes_cpp::ParameterBlackboard',
                    name='trial',
                    parameters=[{'param_1':5}]
                )
            ],
            output='screen',
    )

    return launch.LaunchDescription([
        GroupAction(
            actions = [
                #SetParametersFromFile('./eg.yaml'),
                #SetParametersFromFile('eg.yaml'),
                SetParametersFromFile('/home/aditya/Desktop/cmp_nodes/eg.yaml'),
                SetParameter(name='param_2', value=10),
                container
            ]
        )
    ])

TODO :

  • Add test cases

Signed-off-by: Aditya Pande aditya050995@gmail.com

Signed-off-by: Aditya Pande <aditya050995@gmail.com>
Copy link
Member

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with this TODO resolved:

# TODO: (Aditya) Add test case for composable node

Signed-off-by: Aditya Pande <aditya050995@gmail.com>
Signed-off-by: Aditya Pande <aditya050995@gmail.com>
@adityapande-1995
Copy link
Contributor Author

adityapande-1995 commented Nov 1, 2021

CI :
build : --packages-above-and-dependencies launch_ros
test : --packages-above launch_ros

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

Copy link
Member

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@ivanpauno ivanpauno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with green CI

launch_ros/launch_ros/actions/load_composable_nodes.py Outdated Show resolved Hide resolved
Signed-off-by: Aditya Pande <aditya050995@gmail.com>
@adityapande-1995
Copy link
Contributor Author

adityapande-1995 commented Nov 2, 2021

CI round 2 :
build : --packages-above-and-dependencies launch_ros
test : --packages-above launch_ros

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows [Build Status]

Windows failures seem to be unrelated : https://ci.ros2.org/view/All/job/ci_windows/15761/#showFailuresLink

CI rerun for windows : Build Status

@adityapande-1995
Copy link
Contributor Author

Looks life a flaky failure. @jacobperron should I merge this ?

@jacobperron
Copy link
Member

LGTM, go ahead and merge 🎉

@adityapande-1995 adityapande-1995 merged commit 6410f6c into master Nov 2, 2021
@delete-merged-branch delete-merged-branch bot deleted the aditya/set_params_from_file_composable_nodes branch November 2, 2021 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants